Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: Add echo entity to MPM/MTS #1184

Merged
merged 1 commit into from
Feb 9, 2021
Merged

FIX: Add echo entity to MPM/MTS #1184

merged 1 commit into from
Feb 9, 2021

Conversation

effigies
Copy link
Collaborator

@effigies effigies commented Feb 9, 2021

By my reading of anat.yaml, this needs fixing. Also leading to an error in https://github.com/effigies/BIDS-examples/tree/qmri/qmri_mpm.

@effigies effigies requested a review from tsalo February 9, 2021 21:32
Copy link
Member

@tsalo tsalo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Not sure why the test is failing though.

@effigies
Copy link
Collaborator Author

effigies commented Feb 9, 2021

Yeah, I restarted it and it still fails. @rwblair any ideas? It's possible that some dependency just made a release and broke everything. Will know that when you update #1183.

@effigies
Copy link
Collaborator Author

effigies commented Feb 9, 2021

Just searching, we might need to look into something like jestjs/jest#8384 (comment) or jestjs/jest#9881 (comment).

@nellh
Copy link
Member

nellh commented Feb 9, 2021

Yeah, this looks like it's just inconsistent performance on Circle. A higher timeout can be passed as the third argument to the it() call - the two node spawn tests that reference the valid_headers dir should just have the timeout raised. It's probably slow just because it ends up accessing more of node_modules in the new node process than the other tests.

@codecov
Copy link

codecov bot commented Feb 9, 2021

Codecov Report

Merging #1184 (441a88c) into master (96a7104) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1184   +/-   ##
=======================================
  Coverage   79.05%   79.05%           
=======================================
  Files          78       78           
  Lines        2617     2617           
  Branches      598      598           
=======================================
  Hits         2069     2069           
  Misses        407      407           
  Partials      141      141           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 96a7104...441a88c. Read the comment docs.

@effigies effigies merged commit 49810ac into master Feb 9, 2021
@effigies effigies deleted the fix/mpm_echo branch February 9, 2021 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants